-
Notifications
You must be signed in to change notification settings - Fork 973
Conversation
f376e66
to
8797bfc
Compare
8797bfc
to
6c27646
Compare
test/tab-components/tabPagesTest.js
Outdated
.changeSetting(settings.TABS_PER_PAGE, 1) | ||
.waitForElementCount(tabPage, defaultTabsPerPage) | ||
.waitForBrowserWindow() | ||
.loadUrl('about:preferences') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not loaded in the right context, but I am not sure how to switch to the right one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tabByIndex?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Problem is that is not working for me. Most of the times we use if with 0, maybe 1. But my use case is that I need to focus last active tab, which is this case 19 (we have 20 tabs). But if do .tabByIndex(19)
second tab is selected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just pushed new version with tabByIndex(19)
so you can try it out. I am thinking that something is not ok with handles
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test fixed based on @cezaraugusto findings
0474bde
to
a0b1a86
Compare
fb6a002
to
e6daef6
Compare
Resolves brave#7806 Auditors: @bsclifton @cezaraugusto Test Plan: - Open about:preferences#tabs - Set the number of tabs per tab set to 6 - Open new tabs to create a new tab set, which has just 2 tabs - Increase the number to 8
e6daef6
to
653babb
Compare
@cezaraugusto PR is now ready for a review, test was fixed based on your discovery. Thank you |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the test plan works.
@luixxiul thank you for your test |
Submitter Checklist:
git rebase -i
to squash commits (if needed).Resolves #7806
Auditors: @bsclifton @cezaraugusto
Test Plan:
Reviewer Checklist:
Tests